Review changes with  

Changed Files

File	Status
core/src/code/mod.rs	7% smaller
Cargo.lock	Unsupported file format
core/Cargo.toml	Unsupported file format
core/src/code/utils.rs	0% smaller
gsdk/src/rpc.rs	0% smaller
gsdk/tests/rpc.rs	0% smaller
node/service/src/rpc/mod.rs	0% smaller
node/service/src/rpc/wasm_section.rs	0% smaller

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new node-level RPC endpoint designed to improve developer experience by allowing clients to fetch custom sections from on-chain WASM programs. This is particularly useful for retrieving embedded metadata, such as Sails IDL, without requiring external build artifacts. The implementation includes robust utility functions for WASM parsing and ensures compatibility with historical block queries.

Highlights

• New RPC Endpoint: Added the gear_readWasmCustomSection RPC endpoint to allow clients to retrieve custom sections from on-chain WASM code.
• Utility Implementation: Implemented get_custom_section_data in gear-core to safely extract specific WASM sections, supported by comprehensive unit tests.
• Dependency Update: Added wasm-encoder to the project dependencies to facilitate WASM manipulation in tests.

@claude opus review

Claude finished @ukint-vs's task in 3m 38s —— View job

Claude Opus Review

The PR adds a node-service-level RPC gear_readWasmCustomSection that reads the on-chain original WASM for a given code_id, parses out a named custom section, and returns its raw bytes. The storage-prefix construction mirrors the existing pattern in pallets/gear/rpc/src/lib.rs:188, and the parsing logic is split into a reusable helper in gear-core. Overall the change is self-contained, read-only, introduces no runtime/consensus behavior, and is a reasonable way to expose embedded metadata (notably sails:idl) without a runtime API bump.

Findings below.

severity: medium — saw_version heuristic re-implements logic that wasmparser already provides, and the &'static str error throws away useful diagnostics.

File: core/src/code/utils.rs lines 545–568

pub fn get_custom_section_data<'a>(
    wasm: &'a [u8],
    section_name: &str,
) -> Result<Option<&'a [u8]>, &'static str> {
    let parser = wasmparser::Parser::new(0);
    let mut saw_version = false;

    for payload in parser.parse_all(wasm) {
        let payload = payload.map_err(|_| "invalid wasm binary")?;
        match payload {
            Payload::Version { .. } => saw_version = true,
            Payload::CustomSection(section) if section.name() == section_name => {
                return Ok(Some(section.data()));
            }
            _ => {}
        }
    }

    if !saw_version {
        return Err("invalid wasm binary");
    }

    Ok(None)
}

Two issues bundled:

1. wasmparser already exposes a sniff/validation shortcut (this is what @StackOverflowExcept1on pointed to). Checking the magic + version up front is cleaner than threading a saw_version boolean through the payload loop — that pattern mixes "is this a wasm file at all?" with "is the section present?" concerns.
2. The &'static str error discards the underlying BinaryReaderError detail that .map_err(|_| ...) throws away. Since the only caller surfaces this string as RPC error data, losing the parser's offset/reason makes on-chain corruption (in the unlikely event) much harder to diagnose. Returning CodeError::Validation(_) or at least a String carrying format!("{err}") would preserve the information without any callsite change.

Fix this →

severity: medium — verification gap: no end-to-end test for the RPC.

The 5 unit tests cover get_custom_section_data on synthetic WASM, which is useful but tests only the helper. Things not exercised by any automated test:

• Storage key construction (prefix + Identity(code_id)) — a silent mismatch with the pallet's storage definition would make the RPC always return null. That's exactly the failure mode @gemini-code-assist worried about; the only thing protecting us today is manual inspection.
• SCALE-decoding of the stored Vec<u8> envelope in wasm_section.rs:100.
• Historical block query via at — flagged as unchecked in the PR's own test plan.
• Return shape over the wire (Option<Bytes> vs null).

Given the PR's test plan has two items unchecked, I'd suggest at minimum a gsdk/integration test that uploads a code blob with a known custom section and round-trips it via the RPC. This would catch any future pallet storage rename or hasher change immediately, rather than at the next manual smoke test.

severity: low — tests use fully-qualified wasm_encoder::* paths (also flagged by @StackOverflowExcept1on).

File: core/src/code/mod.rs lines 1253–1309

let mut module = wasm_encoder::Module::new();
let mut types = wasm_encoder::TypeSection::new();
types.ty().function(vec![], vec![]);
module.section(&types);
module.section(&wasm_encoder::CustomSection {
    name: name.into(),
    data: data.into(),
});

Per rustfmt.toml imports_granularity = "Crate" and general convention in this repo, add use wasm_encoder::{Module, TypeSection, CustomSection}; at the top of the test module and drop the wasm_encoder:: prefixes.

severity: low — minor: first_match_returned test duplicates the body of make_wasm_with_custom_section.

File: core/src/code/mod.rs lines 1279–1296

The helper make_wasm_with_custom_section is inlined again because the test needs two custom sections. Either extend the helper to accept a slice of (name, data) pairs, or at minimum consolidate after the import cleanup above — the current duplication will drift if the skeleton module changes.

Notes / non-findings

• Storage prefix fragility (raised by @gemini-code-assist, responded to by author): the GearProgram prefix is correct and matches existing usage. The Identity hasher coupling on CodeId is inherent to how OriginalCodeStorage is defined (pallets/gear-program/src/lib.rs:231) and the same pattern is used by the existing Gear RPC; acceptable as-is.
• Error code 9000 matches the convention used by node/service/src/rpc/runtime_info.rs:99 for other node-service RPCs — consistent.
• wasm-encoder is added as [dev-dependencies] — good, it does not ship in runtime builds.
• No consensus / storage / runtime-API impact. This is purely a read-only node-service RPC.

Summary

Overall the change is sound and follows existing conventions. The most meaningful improvement would be an integration test that exercises the full RPC path, because the helper-level unit tests can't catch a storage-key or decoding regression. Secondary: clean up the saw_version heuristic and preserve parser error detail.
· vs/read-wasm-custom-section

/review